kvm: add SCSI controllers based on the number of virtio-SCSI disks #9823

weizhouapache · 2024-10-17T12:10:17Z

Description

This PR fixes #7490

According to libvirt code, the units per scsi controller is set to 7
therefore, we need to create scsi controller every 7 disks (including CDROM).

https://github.com/libvirt/libvirt/blob/50cc7a0d9d2b9df085ec073a6d60820a9642158a/src/conf/domain_conf.h#L3007-L3008

https://github.com/libvirt/libvirt/blob/50cc7a0d9d2b9df085ec073a6d60820a9642158a/src/conf/domain_conf.c#L6701-L6704

Steps to reproduce the issue

register a template or update template guest os type to "Other PV (VirtIO SCSI)"
create vm
attach 6 disks. check vm xml dump, it looks good

    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' cache='none' discard='unmap'/>
      <source file='/mnt/3bdc199b-d6f8-3a19-a139-3b0ae09379d5/e613dd6e-e0df-41ba-bf24-75bc0c5d4771' index='8'/>
      <backingStore/>
      <target dev='sdg' bus='scsi'/>
      <serial>e613dd6ee0df41babf24</serial>
      <alias name='scsi0-0-0-6'/>
      <address type='drive' controller='0' bus='0' target='0' unit='6'/>
    </disk>
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' cache='none' discard='unmap'/>
      <source file='/mnt/c8c866fd-5553-3dc7-99a2-6df2a6ff516e/6d21d6ec-7f0f-4acb-abe4-ae4da1f1e83e' index='9'/>
      <backingStore/>
      <target dev='sdh' bus='scsi'/>
      <serial>6d21d6ec7f0f4acbabe4</serial>
      <alias name='scsi1-0-0-0'/>
      <address type='drive' controller='1' bus='0' target='0' unit='0'/>
    </disk>
...
    <controller type='scsi' index='0' model='virtio-scsi'>
      <driver queues='1'/>
      <alias name='scsi0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
    </controller>
    <controller type='scsi' index='1' model='virtio-scsi'>
      <alias name='scsi1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
    </controller>

stop and start the vm

Prior to this PR, a scsi controller with model lsilogic is created automatically

    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' cache='none' discard='unmap'/>
      <source file='/mnt/c8c866fd-5553-3dc7-99a2-6df2a6ff516e/6d21d6ec-7f0f-4acb-abe4-ae4da1f1e83e' index='2'/>
      <backingStore/>
      <target dev='sdh' bus='scsi'/>
      <serial>6d21d6ec7f0f4acbabe4</serial>
      <alias name='scsi1-0-0'/>
      <address type='drive' controller='1' bus='0' target='0' unit='0'/>
    </disk>
...
    <controller type='scsi' index='0' model='virtio-scsi'>
      <driver queues='1'/>
      <alias name='scsi0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
    </controller>
...
    <controller type='scsi' index='1' model='lsilogic'>
      <alias name='scsi1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
    </controller>

With this PR, a scsi controller with model virtio-scsi is created automatically

    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' cache='none' discard='unmap'/>
      <source file='/mnt/c8c866fd-5553-3dc7-99a2-6df2a6ff516e/6d21d6ec-7f0f-4acb-abe4-ae4da1f1e83e' index='2'/>
      <backingStore/>
      <target dev='sdh' bus='scsi'/>
      <serial>6d21d6ec7f0f4acbabe4</serial>
      <alias name='scsi1-0-0-0'/>
      <address type='drive' controller='1' bus='0' target='0' unit='0'/>
    </disk>
......
    <controller type='scsi' index='0' model='virtio-scsi'>
      <driver queues='1'/>
      <alias name='scsi0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
    </controller>
    <controller type='scsi' index='1' model='virtio-scsi'>
      <driver queues='1'/>
      <alias name='scsi1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
    </controller>

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

According to libvirt code, the units per scsi controller is set to 7 therefore, we need to create scsi controller every 7 disks (including CDROM). https://github.com/libvirt/libvirt/blob/50cc7a0d9d2b9df085ec073a6d60820a9642158a/src/conf/domain_conf.h#L3007-L3008 https://github.com/libvirt/libvirt/blob/50cc7a0d9d2b9df085ec073a6d60820a9642158a/src/conf/domain_conf.c#L6701-L6704

weizhouapache · 2024-10-17T12:14:43Z

@blueorangutan package

blueorangutan · 2024-10-17T12:16:04Z

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

codecov · 2024-10-17T12:58:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.64%. Comparing base (1af4158) to head (89eb4eb).
Report is 821 commits behind head on 4.19.

Additional details and impacted files

@@              Coverage Diff              @@
##               4.19    #9823       +/-   ##
=============================================
+ Coverage     12.27%   15.64%    +3.36%     
- Complexity     9335    12731     +3396     
=============================================
  Files          4699     5416      +717     
  Lines        414691   528436   +113745     
  Branches      53409    79628    +26219     
=============================================
+ Hits          50891    82657    +31766     
- Misses       357475   436670    +79195     
- Partials       6325     9109     +2784

Flag	Coverage Δ
uitests	`4.30% <ø> (?)`
unittests	`16.31% <100.00%> (+4.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

blueorangutan · 2024-10-17T13:07:50Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11381

weizhouapache · 2024-10-17T13:09:41Z

@blueorangutan test

blueorangutan · 2024-10-17T13:12:03Z

@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2024-10-18T02:48:46Z

[SF] Trillian test result (tid-11684)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 46725 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9823-t11684-kvm-ol8.zip
Smoke tests completed. 110 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestISOUsage>:setup	`Error`	0.00	test_usage.py

DaanHoogland

clgtm

...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

weizhouapache · 2024-10-28T15:38:30Z

@blueorangutan package

blueorangutan · 2024-10-28T15:40:04Z

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-10-28T16:30:56Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11459

rohityadavcloud · 2024-11-07T09:17:32Z

@blueorangutan test

blueorangutan · 2024-11-07T09:18:03Z

@rohityadavcloud a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2024-11-07T23:01:45Z

[SF] Trillian test result (tid-11762)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 47024 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9823-t11762-kvm-ol8.zip
Smoke tests completed. 110 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestISOUsage>:setup	`Error`	0.00	test_usage.py

rohityadavcloud · 2024-12-03T06:47:38Z

@blueorangutan package

blueorangutan · 2024-12-03T06:48:04Z

@rohityadavcloud a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-12-03T07:47:06Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11686

blueorangutan · 2024-12-10T10:06:51Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11769

blueorangutan · 2024-12-11T01:33:50Z

[SF] Trillian test result (tid-11878)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 50690 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9823-t11878-kvm-ol8.zip
Smoke tests completed. 130 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_secure_vm_migration	`Error`	134.39	test_vm_life_cycle.py
test_01_secure_vm_migration	`Error`	134.39	test_vm_life_cycle.py
test_02_redundant_VPC_default_routes	`Failure`	387.37	test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nics	`Failure`	383.67	test_vpc_redundant.py
test_01_redundant_vpc_site2site_vpn	`Failure`	409.59	test_vpc_vpn.py

vladimirpetrov

LGTM based on manual testing.

vladimirpetrov · 2025-01-21T22:21:40Z

@blueorangutan test

blueorangutan · 2025-01-21T22:24:03Z

@vladimirpetrov a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-01-22T11:59:20Z

[SF] Trillian test result (tid-12158)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 46520 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9823-t12158-kvm-ol8.zip
Smoke tests completed. 109 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestISOUsage>:setup	`Error`	0.00	test_usage.py
test_01_migrate_VM_and_root_volume	`Error`	83.54	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	51.94	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	0.07	test_vm_life_cycle.py

DaanHoogland · 2025-01-22T12:59:44Z

the usage error is failed to download iso, which is not related.
the life-cyle ones are intermitted errors

weizhouapache · 2025-01-22T13:25:24Z

the usage error is failed to download iso, which is not related. the life-cyle ones are intermitted errors

thanks @vladimirpetrov @DaanHoogland

…pache#9823) According to libvirt code, the units per scsi controller is set to 7 therefore, we need to create scsi controller every 7 disks (including CDROM). https://github.com/libvirt/libvirt/blob/50cc7a0d9d2b9df085ec073a6d60820a9642158a/src/conf/domain_conf.h#L3007-L3008 https://github.com/libvirt/libvirt/blob/50cc7a0d9d2b9df085ec073a6d60820a9642158a/src/conf/domain_conf.c#L6701-L6704

boring-cyborg bot added the component:kvm label Oct 17, 2024

weizhouapache linked an issue Oct 17, 2024 that may be closed by this pull request

Data Disks get unavailable when VM is shut down #7490

Closed

weizhouapache mentioned this pull request Oct 17, 2024

Data Disks get unavailable when VM is shut down #7490

Closed

rohityadavcloud approved these changes Oct 17, 2024

View reviewed changes

rohityadavcloud added this to the 4.19.2.0 milestone Oct 17, 2024

weizhouapache marked this pull request as ready for review October 17, 2024 14:04

DaanHoogland approved these changes Oct 28, 2024

View reviewed changes

...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java Outdated Show resolved Hide resolved

Update pr9823: add new method

89eb4eb

DaanHoogland added the status:needs-testing label Nov 8, 2024

DaanHoogland assigned vladimirpetrov Nov 12, 2024

rohityadavcloud changed the base branch from 4.18 to 4.19 December 3, 2024 06:47

rohityadavcloud closed this Dec 3, 2024

rohityadavcloud reopened this Dec 3, 2024

vladimirpetrov removed their assignment Dec 11, 2024

vladimirpetrov self-requested a review January 20, 2025 14:01

vladimirpetrov self-assigned this Jan 20, 2025

vladimirpetrov approved these changes Jan 21, 2025

View reviewed changes

vladimirpetrov removed their assignment Jan 21, 2025

vladimirpetrov removed the status:needs-testing label Jan 21, 2025

DaanHoogland merged commit b186272 into apache:4.19 Jan 22, 2025
47 of 50 checks passed

DaanHoogland deleted the 4.18-kvm-add-virtio-scsi-controller branch January 22, 2025 13:00

weizhouapache mentioned this pull request Oct 1, 2025

Unable to add more than 2 volumes or networks to a vm deployed from a UEFI image #11212

Open

gw769 mentioned this pull request Nov 18, 2025

[CloudStack 4.20.1.0]Extra lsilogic SCSI controller on aarch64 causes last disk unrecognized #12090

Open

kvm: add SCSI controllers based on the number of virtio-SCSI disks #9823

kvm: add SCSI controllers based on the number of virtio-SCSI disks #9823

Uh oh!

Conversation

weizhouapache commented Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

weizhouapache commented Oct 17, 2024

Uh oh!

blueorangutan commented Oct 17, 2024

Uh oh!

codecov bot commented Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented Oct 17, 2024

Uh oh!

weizhouapache commented Oct 17, 2024

Uh oh!

blueorangutan commented Oct 17, 2024

Uh oh!

blueorangutan commented Oct 18, 2024

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weizhouapache commented Oct 28, 2024

Uh oh!

blueorangutan commented Oct 28, 2024

Uh oh!

blueorangutan commented Oct 28, 2024

Uh oh!

rohityadavcloud commented Nov 7, 2024

Uh oh!

blueorangutan commented Nov 7, 2024

Uh oh!

blueorangutan commented Nov 7, 2024

Uh oh!

rohityadavcloud commented Dec 3, 2024

Uh oh!

blueorangutan commented Dec 3, 2024

Uh oh!

blueorangutan commented Dec 3, 2024

Uh oh!

blueorangutan commented Dec 10, 2024

Uh oh!

blueorangutan commented Dec 11, 2024

Uh oh!

vladimirpetrov left a comment

Choose a reason for hiding this comment

Uh oh!

vladimirpetrov commented Jan 21, 2025

Uh oh!

blueorangutan commented Jan 21, 2025

Uh oh!

blueorangutan commented Jan 22, 2025

Uh oh!

DaanHoogland commented Jan 22, 2025

Uh oh!

Uh oh!

weizhouapache commented Jan 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

weizhouapache commented Oct 17, 2024 •

edited

Loading

codecov bot commented Oct 17, 2024 •

edited

Loading